Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

acctests: Ignore data sources in imported state in ImportState steps #1074

Closed
wants to merge 2 commits into from

Conversation

gdavison
Copy link
Contributor

@gdavison gdavison commented Oct 4, 2022

Starting with Terraform v1.3, the terraform import command populates an empty state file with the imported resource and any data sources. Previously, only the target resource was written to the state file. This causes acceptance test steps with ImportState to fail if the configuration contains any data sources.

Excludes data sources from both original state and imported state

Closes #1073
Closes #1066

@gdavison gdavison marked this pull request as ready for review October 5, 2022 18:39
@gdavison gdavison requested a review from a team as a code owner October 5, 2022 18:39
@gdavison gdavison changed the title acctests: Ignore data sources in imported state in ImportStateVerify steps acctests: Ignore data sources in imported state in ImportState steps Oct 6, 2022
Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @gdavison.
Could you possibly add a test, perhaps to teststep_providers_test.go, to verify the behaviour? Thanks.

@bflad bflad added bug Something isn't working subsystem/tests Issues and feature requests related to the testing framework. labels Oct 10, 2022
@bflad bflad added this to the v2.24.0 milestone Oct 10, 2022
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For additional context, this change is more permanent for Terraform version 1.3 so this type of change seems appropriate.

Looks good to me once testing, such as #1077, is also included. 🚀

@@ -26,7 +26,7 @@ require (
github.com/mitchellh/reflectwalk v1.0.2
github.com/zclconf/go-cty v1.11.0
golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167
golang.org/x/tools v0.0.0-20200713011307-fd294ab11aed
golang.org/x/tools v0.1.12
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this change required for some reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not directly related to this fix, but was required to run make generate which is a dependency for make test.

bendbennett added a commit that referenced this pull request Oct 13, 2022
…tate (#1077)

* Adding test coverage for changes in #1074 (#1066)

* Skips data sources from both original state and imported state

* Adding changelog entry (#1066)

Co-authored-by: Graham Davison <[email protected]>
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working subsystem/tests Issues and feature requests related to the testing framework.
Projects
None yet
4 participants